Add support for iae_post and iae_post.jwt response mode#183
Add support for iae_post and iae_post.jwt response mode#183swati-technocratblues wants to merge 1 commit into
Conversation
* Add OpenID4VP response mode handling changes * Address PR review comments * Address PR review comments
WalkthroughAdds two new response modes, ChangesIAE_POST/IAE_POST_JWT Response Mode Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
kotlin/openID4VP/src/commonTest/kotlin/io/mosip/openID4VP/responseModeHandler/ResponseModeBasedHandlerFactoryTest.kt (1)
44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the matching
IAE_POSTfactory assertion too.This PR adds both
IAE_POSTandIAE_POST_JWTbranches inResponseModeBasedHandlerFactory, but this file only adds coverage for the JWT path. The new non-JWT mapping can still regress unnoticed.Proposed test addition
+ `@Test` + fun `get should return DirectPostResponseModeHandler for iae_post mode`() { + val handler = ResponseModeBasedHandlerFactory.get(ResponseMode.IAE_POST.value) + + assertTrue(handler is DirectPostResponseModeHandler) + assertNotNull(handler) + } + `@Test` fun `get should return DirectPostJwtResponseModeHandler for iae_post_jwt mode`() { val handler = ResponseModeBasedHandlerFactory.get(ResponseMode.IAE_POST_JWT.value)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@kotlin/openID4VP/src/commonTest/kotlin/io/mosip/openID4VP/responseModeHandler/ResponseModeBasedHandlerFactoryTest.kt` around lines 44 - 50, Add test coverage for the new non-JWT response mode mapping in ResponseModeBasedHandlerFactory by extending ResponseModeBasedHandlerFactoryTest with an assertion that get returns the correct handler for ResponseMode.IAE_POST.value, alongside the existing IAE_POST_JWT test. Use the existing get method and the handler type expected for the IAE_POST branch so both factory paths are validated and future regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@kotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/authorizationRequest/presentationDefinition/PresentationDefinitionUtil.kt`:
- Around line 155-157: The validation in PresentationDefinitionUtil should use
the actual response mode literal supported by ResponseMode.IAE_POST_JWT.value,
since the current exception text mentions an unsupported `iae_post_jwt` value.
Update the message in the mso_mdoc guard to reference the exact supported
literal used by the enum, alongside the existing `direct_post.jwt` and
`iar-post.jwt` values, so callers see a consistent contract.
---
Nitpick comments:
In
`@kotlin/openID4VP/src/commonTest/kotlin/io/mosip/openID4VP/responseModeHandler/ResponseModeBasedHandlerFactoryTest.kt`:
- Around line 44-50: Add test coverage for the new non-JWT response mode mapping
in ResponseModeBasedHandlerFactory by extending
ResponseModeBasedHandlerFactoryTest with an assertion that get returns the
correct handler for ResponseMode.IAE_POST.value, alongside the existing
IAE_POST_JWT test. Use the existing get method and the handler type expected for
the IAE_POST branch so both factory paths are validated and future regressions
are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47c52ea5-4e51-4c74-912d-94b4a44461c3
📒 Files selected for processing (6)
kotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/authorizationRequest/authorizationRequestHandler/types/RedirectUriPrefixAuthorizationRequestHandler.ktkotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/authorizationRequest/presentationDefinition/PresentationDefinitionUtil.ktkotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/constants/ResponseMode.ktkotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/responseModeHandler/ResponseModeBasedHandlerFactory.ktkotlin/openID4VP/src/commonTest/kotlin/io/mosip/openID4VP/authorizationRequest/authorizationRequestHandler/types/RedirectUriPrefixAuthorizationRequestHandlerTest.ktkotlin/openID4VP/src/commonTest/kotlin/io/mosip/openID4VP/responseModeHandler/ResponseModeBasedHandlerFactoryTest.kt
| if (hasMsoMdocFormat && (responseMode != ResponseMode.DIRECT_POST_JWT.value && responseMode != ResponseMode.IAR_POST_JWT.value && | ||
| responseMode != ResponseMode.IAE_POST_JWT.value)) { | ||
| throw OpenID4VPExceptions.InvalidData("When mso_mdoc format is present in presentation definition, response_mode must be direct_post.jwt or iar-post.jwt or iae_post_jwt", className) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the actual iae_post.jwt literal in the exception text.
The new guard accepts ResponseMode.IAE_POST_JWT.value, but the message says iae_post_jwt. That points callers to a value the library does not support.
Proposed fix
- throw OpenID4VPExceptions.InvalidData("When mso_mdoc format is present in presentation definition, response_mode must be direct_post.jwt or iar-post.jwt or iae_post_jwt", className)
+ throw OpenID4VPExceptions.InvalidData(
+ "When mso_mdoc format is present in presentation definition, response_mode must be direct_post.jwt or iar-post.jwt or iae_post.jwt",
+ className
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (hasMsoMdocFormat && (responseMode != ResponseMode.DIRECT_POST_JWT.value && responseMode != ResponseMode.IAR_POST_JWT.value && | |
| responseMode != ResponseMode.IAE_POST_JWT.value)) { | |
| throw OpenID4VPExceptions.InvalidData("When mso_mdoc format is present in presentation definition, response_mode must be direct_post.jwt or iar-post.jwt or iae_post_jwt", className) | |
| if (hasMsoMdocFormat && (responseMode != ResponseMode.DIRECT_POST_JWT.value && responseMode != ResponseMode.IAR_POST_JWT.value && | |
| responseMode != ResponseMode.IAE_POST_JWT.value)) { | |
| throw OpenID4VPExceptions.InvalidData( | |
| "When mso_mdoc format is present in presentation definition, response_mode must be direct_post.jwt or iar-post.jwt or iae_post.jwt", | |
| className | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@kotlin/openID4VP/src/commonMain/kotlin/io/mosip/openID4VP/authorizationRequest/presentationDefinition/PresentationDefinitionUtil.kt`
around lines 155 - 157, The validation in PresentationDefinitionUtil should use
the actual response mode literal supported by ResponseMode.IAE_POST_JWT.value,
since the current exception text mentions an unsupported `iae_post_jwt` value.
Update the message in the mso_mdoc guard to reference the exact supported
literal used by the enum, alongside the existing `direct_post.jwt` and
`iar-post.jwt` values, so callers see a consistent contract.
| assertNotNull(handler) | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Add a test case for iae_post return DirectPostResponseModeHandler
Summary by CodeRabbit
New Features
iae_postandiae_post.jwt.Bug Fixes
mso_mdoc, allowing the newly supported IAE JWT response mode.Tests